-
Notifications
You must be signed in to change notification settings - Fork 1
fix(ErrorMessage): add component name in log message #158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments about forms and homogeneity of messages. I leave @MelchiorSchuh giving its opinion on changes
src/geode/inspector/criterion/adjacency/brep_meshes_adjacency.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/adjacency/brep_meshes_adjacency.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/colocation/component_meshes_colocation.cpp
Show resolved
Hide resolved
src/geode/inspector/criterion/internal/component_meshes_degeneration.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/intersections/model_intersections.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/negative_elements/solid_negative_elements.cpp
Outdated
Show resolved
Hide resolved
| { | ||
| InspectionIssues< index_t > wrong_polygons{ "Negative Polygons." }; | ||
| InspectionIssues< index_t > wrong_polygons{ | ||
| "Negative Area Polygons" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General comments:
- I prefer uuids of components given directly after name rather than at line end
- each line should be informative on its own, if there are 200 lines of polygon adjacency issues, having to go up the stack to know what the issue is about is a problem
- Fix a convention for upper cases at the beginning of issue strings and apply it everywhere. I would upper-case Corner, Line, Surface, Block and lower case the rest.
src/geode/inspector/criterion/intersections/model_intersections.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/intersections/surface_curve_intersections.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/intersections/surface_intersections.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/manifold/solid_facet_manifold.cpp
Outdated
Show resolved
Hide resolved
include/geode/inspector/criterion/colocation/component_meshes_colocation.hpp
Outdated
Show resolved
Hide resolved
include/geode/inspector/criterion/colocation/unique_vertices_colocation.hpp
Outdated
Show resolved
Hide resolved
include/geode/inspector/criterion/colocation/unique_vertices_colocation.hpp
Outdated
Show resolved
Hide resolved
include/geode/inspector/criterion/degeneration/brep_meshes_degeneration.hpp
Outdated
Show resolved
Hide resolved
include/geode/inspector/criterion/degeneration/brep_meshes_degeneration.hpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/intersections/model_intersections.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/intersections/model_intersections.cpp
Outdated
Show resolved
Hide resolved
src/geode/inspector/criterion/manifold/solid_vertex_manifold.cpp
Outdated
Show resolved
Hide resolved
|
🎉 This PR is included in version 6.8.5-rc.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 6.8.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.